Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Wallet] move wallet help string creation to CWallet #7576

Merged
merged 1 commit into from Mar 11, 2016

Conversation

jonasschnelli
Copy link
Contributor

Simple and easy-to-review refactoring to reduce wallets init.cpp polution and sets another step in the wallet separation / duplication.

The only visible change to users would be, that the wallet help message part has now it's own -help-debug part (IMO desirable if we look at the wallet as it is a module)

@paveljanik
Copy link
Contributor

ACK (please fix the typo).

@kirkalx
Copy link
Contributor

kirkalx commented Feb 22, 2016

Nice, utACK.

@maflcko
Copy link
Member

maflcko commented Feb 26, 2016

Looks good. utACK acf7f87

@@ -55,6 +55,8 @@ static const unsigned int DEFAULT_TX_CONFIRM_TARGET = 2;
static const unsigned int MAX_FREE_TRANSACTION_CREATE_SIZE = 1000;
static const bool DEFAULT_WALLETBROADCAST = true;

static const char * const DEFAULT_WALLET_DAT = "wallet.dat";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better not to put string constants in header files. This causes problems, such as duplication - either undetected and wasting space in the executable, or explicit linker duplication errors.

Better to make it "extern const char*" and define the value in the implementation file.

@laanwj
Copy link
Member

laanwj commented Mar 1, 2016

utACK apart from nit - great idea.

@sipa
Copy link
Member

sipa commented Mar 5, 2016

Concept ACK, but fix @laanwj's nit and the typo.

@@ -869,6 +871,9 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface

/* Mark a transaction (and it in-wallet descendants) as abandoned so its inputs may be respent. */
bool AbandonTransaction(const uint256& hashTx);

/* Retruns the wallets help message */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonasschnelli Please address the feedback so this can be merged next week.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Will fix is in the next coupl of hours. Thanks for the reminder.

@jonasschnelli
Copy link
Contributor Author

Fixed nit (amend force push)

@maflcko
Copy link
Member

maflcko commented Mar 5, 2016

utACK 72c2651

@laanwj laanwj merged commit 72c2651 into bitcoin:master Mar 11, 2016
laanwj added a commit that referenced this pull request Mar 11, 2016
72c2651 [Wallet] move wallet help string creation to CWallet (Jonas Schnelli)
codablock pushed a commit to codablock/dash that referenced this pull request Dec 19, 2017
72c2651 [Wallet] move wallet help string creation to CWallet (Jonas Schnelli)
zkbot added a commit to zcash/zcash that referenced this pull request Dec 18, 2019
random-zebra added a commit to PIVX-Project/PIVX that referenced this pull request Aug 25, 2020
7e71759 Wallet: Split main logic from InitLoadWallet into CreateWalletFromFile (random-zebra)
e585dad [Wallet] refactor wallet/init interaction (random-zebra)
49646b2 [Refactor] Nuke zwalletMain global object (random-zebra)
f5f9df9 Wallet: Let the interval-flushing thread figure out the filename (Luke Dashjr)
74445e4 [wallet] Move hardcoded file name out of log messages (random-zebra)
3f1838d [Wallet] optimize return value of InitLoadWallet() (random-zebra)
539dea4 [Wallet] move "load wallet phase" to CWallet (random-zebra)
7644318 [Wallet] move wallet help string creation to CWallet (random-zebra)
a9d69b8 [trivial] Reuse translation and cleanup DEFAULT_* values (random-zebra)
cfd007a [Bug] Omit wallet-related options from -help when wallet not supported (random-zebra)
da642e6 [Refactor] More constant default values cleanup (random-zebra)
a45275a [Refactor] Implement CBaseChainParams::BaseParams(Network) (random-zebra)
09abb98 Constrain constant values to a single location in code (random-zebra)

Pull request description:

  Adapts the following refactoring PRs:

  - bitcoin#6961 Constrain constant values to a single location in code
  - bitcoin#7576 [Wallet] move wallet help string creation to CWallet
  - bitcoin#7577 move "load wallet phase" to CWallet
  - bitcoin#7608 Move hardcoded file name out of log messages
  - bitcoin#7691 refactor wallet/init interaction
  - bitcoin#8776 Wallet refactoring leading up to multiwallet

ACKs for top commit:
  furszy:
    re ACK 7e71759
  Fuzzbawls:
    ACK 7e71759

Tree-SHA512: 5fad328b9ddf8187af97d3d5fb285d0b67e73d51ac1bc44a3d57d0af86bce8b34efaab539b7bdbc4f9a2fa575a936f83788cffcc9c6d6d04cd3e63b19d399400
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants